Improve Dag run metrics - #70013
Conversation
potiuk
left a comment
There was a problem hiding this comment.
Thanks — and you've correctly spotted that the old description was wrong: scheduler.dagruns.running claimed to count "DAGs whose latest DagRun is RUNNING" while the code counted DagRuns outright. Fixing that wording is worth having on its own, and adding a queued counterpart is a reasonable gap to fill.
But the change from a single aggregate gauge to one gauge per dag_id is a much bigger step than the PR presents it as, and I'd like it reconsidered before this lands. Two blocking concerns inline: metric cardinality, and the fact that this silently breaks every existing dashboard built on the old metric.
To be clear about the shape of my objection — I'm not saying per-dag visibility is wrong to want, only that turning it on unconditionally for every deployment is a decision that needs to be deliberate and documented rather than a side effect of adding a queued metric.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
| .where(DagRun.state.in_([DagRunState.RUNNING, DagRunState.QUEUED])) | ||
| .group_by(DagRun.dag_id, DagRun.state) | ||
| ) | ||
| for dag_id, state, count in session.execute(stmt).all(): |
There was a problem hiding this comment.
This emits one gauge per (dag_id, state) pair on every scheduler loop. On a deployment with 10k Dags that's up to 20k time series per interval where there was previously exactly one.
That's a well-known way to overwhelm a StatsD daemon or blow up Prometheus cardinality, and the operators it hits hardest are precisely the large deployments that can least afford it. The blast radius is also invisible from the diff — nothing here scales with deployment size on the page, but it does at runtime.
At minimum this wants to be opt-in behind a config flag (defaulting to the current aggregate behaviour), or to keep emitting the untagged aggregate and add the per-dag breakdown separately. Worth a note in the description about expected cardinality either way.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
There was a problem hiding this comment.
@potiuk Thanks for your review. This is a really good point. I did not thought about that. I added a config flag to enable or disable DAG_ID tags.
| type: "gauge" | ||
| legacy_name: "-" | ||
| name_variables: [] | ||
| legacy_name: "scheduler.dagruns.running.{dag_id}" |
There was a problem hiding this comment.
legacy_name changes from "-" to "scheduler.dagruns.running.{dag_id}", which means on the legacy (non-tagged) StatsD path the metric name itself now embeds the dag_id. Anyone with a dashboard or alert on scheduler.dagruns.running stops receiving that series entirely — it isn't a re-tagging, the old name ceases to exist.
That's a user-visible breaking change to a published metric and needs a newsfragment in airflow-core/newsfragments/ (probably .significant.rst) spelling out what happens to existing dashboards and what to migrate to. There's no newsfragment in the PR at present.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
There was a problem hiding this comment.
@potiuk I addressed this point with changes in stats.py. What do you think? With this update, legacy_name should no longer be an issue when toggling the config flag.
Overview
The scheduler currently only exports scheduler.dagruns.running as a single, untagged count — there's no way to see how many Dag runs are running or queued for a specific Dag. This makes it hard to spot Dags that are backlogged in the QUEUED state (e.g. stuck behind max_active_runs limits or pool exhaustion) or to compare running-vs-queued load per Dag in dashboards/alerts.
This change adds per-dag_id visibility by emitting both scheduler.dagruns.running and a new scheduler.dagruns.queued metric, each tagged with dag_id, so operators can track queue backlog and running by each Dag.
We look forward to your feedback.
Details of change:
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Sonnet 5 following the guidelines